Execution tracing: GraphQL query to get storage inputs for past blocks #2491

Dentosal · 2024-12-11T06:48:27Z

Description

To support local debugging and execution tracing, we need to add an API that returns the state that VM needs to execute transactions. This is done by recording all database accesses during tx validation, called storage read replay (bikeshedding in progress). This data is then exposed as-is through GraphQL. The API is non-trivial to consume, but a seperate client is proviced, see https://github.com/FuelLabs/execution-trace

The implementation exposes database table names and representations directly. Maintaining backwards compatibility with this could turn out to be quite hard.

Follow-ups:

Execution tracing: dry run support #2625

Example query:

mutation {
  storageReadReplay(height:"4") { column, key, value }
}

TODO

Migrate from --debug flag to --historical-execution (see Dry run in the past #2661)

Open questions:

Do we really want --debug flag to enabled this, as opposed to super-high query cost or a separate flag to enable?
What should the query cost be?

Checklist

Breaking changes are clearly marked as such in the PR description and changelog: No breaking changes!
New behavior is reflected in tests
The specification matches the implemented behavior: This is outside spec scope.

Before requesting review

I have reviewed the code myself
I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

See the VM PR FuelLabs/fuel-vm#881.

- Related to #1432 # Release notes In this release, we: - Changed `ABIDecoder` methods to take `std::io::Read` instead of `&[u8]`, allowing it to be used in a streaming manner. # Summary `ABIDecoder` methods take `bytes: impl std::io::Read` instead of `bytes: &[u8]`. This allows decoding abi types without having to know the size in advance. This is particularly useful when reading them directly from VM memory, which will be used by the indexer after FuelLabs/fuel-core#2491 is done. # Breaking Changes `ABIDecoder` methods take `bytes: impl std::io::Read` instead of `bytes: &[u8]`. Callers using arrays or `Vec` must change the argument from `&value` to `value.as_slice()`. # Checklist - [x] All **changes** are **covered** by **tests** (or not applicable) - [x] All **changes** are **documented** (or not applicable) - [x] I **reviewed** the **entire PR** myself (preferably, on GH UI) - [x] I **described** all **Breaking Changes** (or there's none) --------- Co-authored-by: hal3e <[email protected]> Co-authored-by: Ahmed Sagdati <[email protected]> Co-authored-by: segfault-magnet <[email protected]>

AurelienFT

Good integration with database types ! Liked it !

AurelienFT · 2025-01-28T10:52:12Z

crates/fuel-core/src/schema/tx.rs

+        ctx: &Context<'_>,
+        height: U32,
+    ) -> async_graphql::Result<Vec<StorageReadReplayEvent>> {
+        let config = ctx.data_unchecked::<GraphQLConfig>().clone();


Suggested change

let config = ctx.data_unchecked::<GraphQLConfig>().clone();

let config = ctx.data_unchecked::<GraphQLConfig>();

AurelienFT · 2025-01-28T10:59:57Z

crates/services/executor/src/executor.rs

@@ -702,6 +702,114 @@ where
        Ok(())
    }

+    #[tracing::instrument(skip_all)]
+    fn record_storage_reads_for<D>(


This function is never called in the code. Isn't it supposed to be called here ? Or it's outdated ?

Hahahaha I already managed to refactor that code out. Nice. I'll remove it.

AurelienFT

LGTM. I would vouch for an indepedent flag that can be enabled by default when we use debug but that could be enabled by himself also.

tests/tests/storage_read_replay.rs

MitchTurner

I've read a lot more, but I'm still WIP with my review. Will add more feedback.

MitchTurner · 2025-02-06T19:28:12Z

bin/fuel-core/src/cli/run/graphql.rs

+    /// Query costs for generating execution trace for a block.®
+    #[clap(
+        long = "query-cost-storage-read-replay",
+        default_value = DEFAULT_QUERY_COSTS.storage_read_replay.to_string(),
+        env
+    )]
+    pub storage_read_replay: usize,


The verbiage around this isn't clear to me.

This argument is for setting the costs? I think more context is required here and perhaps a more specific name for the field.

Yes. It's a filed in a struct called QueryCosts, and all other arguments have similar purpose.

Ah ah ah. I remember this now. I suggested on another PR that we should rename this to "Complexity" or something (just because we already have concepts of gas costs etc). I won't nitpick too much on this since at least it's within the graphql crate, but giving some clarity in the comment on what types of costs this is would be valuable (even if the other args aren't that clear).

bin/fuel-core/src/cli/run/graphql.rs

MitchTurner

Looking good overall. I add a few easy-to-fix comments.

One thing I'd like to call out is there doesn't seem to be any UTs for the new behavior. I feel like they would be relatively easy to add and would push things left a bit.

tests/tests/storage_read_replay.rs

MitchTurner · 2025-02-07T18:54:39Z

tests/tests/storage_read_replay.rs

+/// Create a counter contract.
+/// Increment it multiple times, and make sure the replay gives correct storage state every time.
+#[tokio::test(flavor = "multi_thread")]
+async fn test_storage_read_replay_returns_counter_state() {


I like tests like this a lot. It's easy to read since the helpers make_counter_contract and increment_counter are very declarative.

It also gives an example of how the chain can be used as you're actually deploying something and interacting with it. Eventually, I want a whole testing framework like this for deploying Sway contracts inside of tests--not for integ tests, but for dApp tests.

tests/tests/storage_read_replay.rs

crates/client/src/client.rs

tests/tests/storage_read_replay.rs

crates/types/Cargo.toml

MitchTurner · 2025-02-07T19:35:37Z

crates/client/src/client/schema/storage_read_replay.rs

+    use fuel_core_types::fuel_types::BlockHeight;
+
+    #[test]
+    fn execution_trace_block_tx_gql_output() {


It would be good to specify that this is a compatibility regression/snapshot test. I don't get anything from this test name.

We have loads of *_gql_output tests that do this. Should those all be renamed?

(changed the method name to match storage_read_replay in fc097bf, as that's what the feature is now called)

We have loads of *_gql_output tests that do this. Should those all be renamed?

Not your responsibility. I don't think upholding the precedence gains us anything here. I'd prefer clear names wherever possible.

Dentosal · 2025-02-07T21:38:47Z

Taking this back to draft until #2661 is merged, as I want to refactor this on top of it.

Dentosal added 2 commits December 11, 2024 08:39

Initial support for historical execution tracing

676a08b

Update schema.sdl

1e018f1

Dentosal added enhancement New feature or request fuel-block-executor wasm WASM-based block execution labels Dec 11, 2024

Dentosal self-assigned this Dec 11, 2024

Dentosal mentioned this pull request Dec 11, 2024

Execution trace recording FuelLabs/fuel-vm#881

Closed

8 tasks

Dentosal and others added 7 commits December 11, 2024 08:50

Fix typo exection => execution

5f6c079

Make execution trace endpoint to work on block level

7c8548f

Fix cli arguments

6ebe050

Various fixes and work towards Rust client support

cd0704a

Merge branch 'master' into dento/execution-trace

63a0016

WIP

f76c96f

Change from tracing to storage read replay recording

3ba2bc7

Dentosal force-pushed the dento/execution-trace branch from 8d80d7b to 3ba2bc7 Compare December 19, 2024 15:18

Dentosal added 2 commits December 30, 2024 14:40

Merge branch 'master' into dento/execution-trace

88a2730

Merge branch 'master' into dento/execution-trace

f558296

Dentosal mentioned this pull request Jan 6, 2025

feat!: streaming decoder FuelLabs/fuels-rs#1564

Merged

4 tasks

rafal-ch added the release label Jan 10, 2025

xgreenx removed the release label Jan 10, 2025

Add wasm executor support

5abde1c

Dentosal changed the title ~~Execution tracing for past blocks~~ Execution tracing: add GraphQL endpoint to get storage inputs for past blocks Jan 15, 2025

Dentosal changed the title ~~Execution tracing: add GraphQL endpoint to get storage inputs for past blocks~~ Execution tracing: GraphQL query to get storage inputs for past blocks Jan 15, 2025

Move behind --debug flag

6827e0d

Dentosal mentioned this pull request Jan 23, 2025

Execution tracing: dry run support #2625

Open

Dentosal added 4 commits January 23, 2025 11:35

Adjust query cost

acb9bab

Add debug flag decription

bb6c9dd

Approve snapshot changes

75ff219

Add a test case

73cb7a6

Dentosal added 3 commits January 25, 2025 01:24

fix typo

31fd109

Add TODO comments for refactoring

a90b559

more clippy

017c03e

Dentosal marked this pull request as ready for review January 24, 2025 23:43

Dentosal requested review from xgreenx, MitchTurner, netrome and acerone85 as code owners January 24, 2025 23:43

Dentosal requested a review from a team January 24, 2025 23:43

Merge branch 'master' into dento/execution-trace

f740d28

AurelienFT self-requested a review January 27, 2025 16:46

AurelienFT reviewed Jan 28, 2025

View reviewed changes

Dentosal added 2 commits January 29, 2025 17:00

Address PR comments

fce82ea

Merge branch 'master' into dento/execution-trace

a4a6bbf

Dentosal requested review from a team and AurelienFT January 29, 2025 15:02

AurelienFT previously approved these changes Jan 30, 2025

View reviewed changes

rymnc reviewed Feb 3, 2025

View reviewed changes

tests/tests/storage_read_replay.rs Outdated Show resolved Hide resolved

rymnc reviewed Feb 3, 2025

View reviewed changes

tests/tests/storage_read_replay.rs Outdated Show resolved Hide resolved

MitchTurner reviewed Feb 6, 2025

View reviewed changes

MitchTurner reviewed Feb 7, 2025

View reviewed changes

Merge branch 'master' into dento/execution-trace

4aa858e

Dentosal dismissed AurelienFT’s stale review via 4aa858e February 7, 2025 20:22

Dentosal requested a review from rafal-ch as a code owner February 7, 2025 20:22

Address PR feedback

fc097bf

Dentosal requested review from MitchTurner, AurelienFT and rymnc February 7, 2025 20:41

Dentosal marked this pull request as draft February 7, 2025 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Execution tracing: GraphQL query to get storage inputs for past blocks #2491

Execution tracing: GraphQL query to get storage inputs for past blocks #2491

Dentosal commented Dec 11, 2024 •

edited

Loading

AurelienFT left a comment

AurelienFT Jan 28, 2025

Dentosal Jan 29, 2025

AurelienFT Jan 28, 2025

Dentosal Jan 29, 2025

Dentosal Jan 29, 2025

AurelienFT left a comment

MitchTurner left a comment

MitchTurner Feb 6, 2025

Dentosal Feb 7, 2025

MitchTurner Feb 7, 2025

MitchTurner left a comment

MitchTurner Feb 7, 2025

MitchTurner Feb 7, 2025

Dentosal Feb 7, 2025

Dentosal Feb 7, 2025

MitchTurner Feb 7, 2025

Dentosal commented Feb 7, 2025

	let config = ctx.data_unchecked::<GraphQLConfig>().clone();
	let config = ctx.data_unchecked::<GraphQLConfig>();

Execution tracing: GraphQL query to get storage inputs for past blocks #2491

Are you sure you want to change the base?

Execution tracing: GraphQL query to get storage inputs for past blocks #2491

Conversation

Dentosal commented Dec 11, 2024 • edited Loading

Description

TODO

Open questions:

Checklist

Before requesting review

After merging, notify other teams

AurelienFT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AurelienFT left a comment

Choose a reason for hiding this comment

MitchTurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MitchTurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dentosal commented Feb 7, 2025

Dentosal commented Dec 11, 2024 •

edited

Loading